VorbisComments: Support current/total TRACKNUMBER fields#500
VorbisComments: Support current/total TRACKNUMBER fields#500Serial-ATA merged 3 commits intoSerial-ATA:mainfrom
Conversation
vdualb
force-pushed
the
main
branch
2 times, most recently
from
6ea135a to
3040351
Compare
Serial-ATA
left a comment
Serial-ATA
left a comment
There was a problem hiding this comment.
Thanks for working on this! Just one suggestion.
| value_split.next().and_then(|b| b.parse().ok()); | ||
| let track_total: Option<u32> = | ||
| value_split.next().and_then(|b| b.parse().ok()); | ||
| if let Some(n) = track_number { |
There was a problem hiding this comment.
The workaround here also mentions vinyl track numbers. I think in the case that the track number doesn't parse, just insert it into the tag like any other item.
There was a problem hiding this comment.
Added vinyl track numbers and another test. Also pushing this tag if track number can't be parsed.
There was a problem hiding this comment.
Sorry for the confusion! I meant Lofty shouldn't do anything special with vinyl tracks, since I imagine there are weird formats for those out in the wild. In the case that no track number is parsed, just insert it into the tag like normal.
Made that change in e961455 (#500)
vdualb
force-pushed
the
main
branch
2 times, most recently
from
12a16c5 to
4a18f79
Compare
Support <current>/<total> and vinyl track number (such as A2, b5) in TRACKNUMBER field Closes: Serial-ATA#499
|
For these kind of unit tests it is not required to add more test files. The tags to be parsed could be constructed programmatically during the tests and we already use this approach in many cases. Using actual test files should be reserverd for testing the (binary) deserialization, not the semantic parsing. |
|
@uklotzde I couldn't immediately figure out how to do that. I will try to change tests later. Could you point to test where tags are constructed programmatically? |
3 participants
Closes: #499